fix: guard weighted-average merge against zero outputAmountSum in historicalOrderCharts by thedavidmeister · Pull Request #2777 · rainlanguage/raindex

thedavidmeister · 2026-06-17T02:04:03Z

Summary

Fixes a divide-by-zero in prepareHistoricalOrderChartData when two or more trades share a timestamp and their outputVaultBalanceChange.amount values sum to zero
When outputAmountSum === 0, falls back to unweighted mean of ioratio values instead of dividing by zero (which would produce NaN or ±Infinity)
Adds an inline vitest case covering the fallback path: two zero-amount trades at the same timestamp → verifies the result is finite and equals the unweighted mean

Note: this is a pure data computation function with no rendering, so a UI screenshot is not applicable.

Co-Authored-By: Claude noreply@anthropic.com

Summary by CodeRabbit

Bug Fixes
- Fixed historical order chart merging for same-timestamp trades by safely handling cases where total output is zero, preventing invalid derived ratios and ensuring the merged chart value resolves to 0 instead of error-prone results.
Tests
- Added Vitest coverage for same-timestamp edge cases where output sums to zero and per-trade ratio inputs are non-finite, asserting the merged chart output is 0.

…toricalOrderCharts When two or more trades share a timestamp and their outputVaultBalanceChange.amount values sum to zero, dividing ioratioSum by outputAmountSum produced NaN/Infinity. Falls back to unweighted mean of ioratio values when outputAmountSum === 0. Closes #2766 Co-Authored-By: Claude <noreply@anthropic.com>

coderabbitai · 2026-06-17T02:04:15Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 40b54f7d-6585-4f17-ab6e-d3cac096ae14

📥 Commits

Reviewing files that changed from the base of the PR and between 671e15f and b3b5538.

📒 Files selected for processing (1)

packages/ui-components/src/lib/services/historicalOrderCharts.ts

📝 Walkthrough

Walkthrough

prepareHistoricalOrderChartData now guards the same-timestamp merge when total output amount is zero, using a fallback mean instead of producing non-finite values. A new Vitest case covers zero-summing output amounts.

Divide-by-zero fix and test

Layer / File(s)	Summary
Zero-output-sum guard and coverage `packages/ui-components/src/lib/services/historicalOrderCharts.ts`	The same-timestamp merge branch now uses an unweighted mean of finite per-trade ratios when `outputAmountSum === 0`, and a new Vitest case asserts the merged chart value is `0` for zero-summing trades.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

rainlanguage/raindex#2763: Related changes in historicalOrderCharts.ts also adjust zero-output handling for chart data and add coverage around non-finite values.

Poem

🐇 I hopped to the chart with a tiny fix in sight,
Zero sums no longer derail the graph tonight.
Finite hops together, neat and clear,
The merge stays steady when zeros appear.
No NaN shadows, no Infinity tide—
Just a bunny-approved mean rolling on with pride.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the divide-by-zero fix in historicalOrderCharts.
Linked Issues check	✅ Passed	The changes address `#2766` by guarding zero output sums and adding a test for the same-timestamp merge case.
Out of Scope Changes check	✅ Passed	The patch stays within the requested data-computation fix and associated test coverage, with no unrelated changes.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch 2026-06-17-issue-2766-historical-chart-divide-zero

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

packages/ui-components/src/lib/services/historicalOrderCharts.ts

Oops! Something went wrong! :(

ESLint: 8.57.1

Error: ESLint configuration in --config is invalid:

Unexpected top-level property "__esModule".

at ConfigValidator.validateConfigSchema (/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2177:19)
at ConfigArrayFactory._normalizeConfigData (/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3019:19)
at ConfigArrayFactory._loadConfigData (/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2984:21)
at ConfigArrayFactory.loadFile (/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2850:40)
at createCLIConfigArray (/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3660:35)
at new CascadingConfigArrayFactory (/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3735:29)
at new CLIEngine (/node_modules/eslint/lib/cli-engine/cli-engine.js:617:36)
at new ESLint (/node_modules/eslint/lib/eslint/eslint.js:430:27)
at Object.execute (/node_modules/eslint/lib/cli.js:410:24)
at async main (/node_modules/eslint/bin/eslint.js:152:22)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

… helper id parameter and orderHash literal in the inline test helper must satisfy the `0x${string}` template literal type required by RaindexTrade. Co-Authored-By: Claude <noreply@anthropic.com>

RaindexTrade is a WASM class with a private constructor and additional required fields (chainId, formattedIoRatio, ioRatio, owner). The inline test helper builds a partial object literal, so cast through unknown rather than enumerate all required properties. Co-Authored-By: Claude <noreply@anthropic.com>

Run prettier on historicalOrderCharts.ts.

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/ui-components/src/lib/services/historicalOrderCharts.ts`:
- Around line 51-55: The fallback calculation for ioratioAverage when
outputAmountSum === 0 does not guard against d.value being non-finite (Infinity
or NaN), which can occur when outputVaultBalanceChange.formattedAmount is "0"
from the initialization at lines 22-25. Filter out non-finite values from the
fallback average calculation by adding a check to ensure each d.value is finite
before including it in the reduce operation. Additionally, update the test
fixture that currently hardcodes "100" to test with "0" values that would expose
this non-finite path and verify the fix handles it correctly.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 68d77525-228b-4304-b28e-e4ae2687eef8

📥 Commits

Reviewing files that changed from the base of the PR and between 175e84a and 671e15f.

📒 Files selected for processing (1)

packages/ui-components/src/lib/services/historicalOrderCharts.ts

When outputVaultBalanceChange.amount is BigInt(0) the formattedAmount is also "0", making the per-trade value Infinity/NaN. The previous fallback divided Infinity × N / N = Infinity. Filter non-finite values before the unweighted mean; if every entry is non-finite (i.e. all output amounts are zero), return 0. Update the test fixture to use formattedAmount:"0" (consistent with amount BigInt(0)) and tighten the assertion to expect value 0. Co-Authored-By: Claude <noreply@anthropic.com>

…-historical-chart-divide-zero

thedavidmeister · 2026-06-26T01:22:21Z

screenshot pending (manual): historicalOrderCharts.ts produces data for lightweight-charts canvas rendering — canvas output requires a headless Chromium harness to screenshot; the change itself is a computation fix (unweighted-mean fallback when outputAmountSum is 0) with no layout change

thedavidmeister self-assigned this Jun 17, 2026

thedavidmeister and others added 3 commits June 17, 2026 13:20

fix(ci): correct 0x-prefixed string types in makeZeroAmountTrade test…

4e8332e

… helper id parameter and orderHash literal in the inline test helper must satisfy the `0x${string}` template literal type required by RaindexTrade. Co-Authored-By: Claude <noreply@anthropic.com>

fix(ci): svelte-lint-format-check [3b-attempt]

671e15f

Run prettier on historicalOrderCharts.ts.

coderabbitai Bot reviewed Jun 17, 2026

View reviewed changes

Comment thread packages/ui-components/src/lib/services/historicalOrderCharts.ts

thedavidmeister and others added 3 commits June 18, 2026 21:31

merge(main): resolve conflicts [merge-update]

d0b97dd

Merge remote-tracking branch 'origin/main' into 2026-06-17-issue-2766…

b3b5538

…-historical-chart-divide-zero

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: guard weighted-average merge against zero outputAmountSum in historicalOrderCharts#2777

fix: guard weighted-average merge against zero outputAmountSum in historicalOrderCharts#2777
thedavidmeister wants to merge 7 commits into
mainfrom
2026-06-17-issue-2766-historical-chart-divide-zero

thedavidmeister commented Jun 17, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 17, 2026 •

edited

Loading

Walkthrough

Estimated code review effort

Possibly related PRs

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

thedavidmeister commented Jun 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

thedavidmeister commented Jun 17, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Estimated code review effort

Possibly related PRs

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

thedavidmeister commented Jun 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

thedavidmeister commented Jun 17, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 17, 2026 •

edited

Loading